Missing compatibility layer functionality #10518
Conversation
|
@julek-wolfssl adding PR for the 3 functions i've seen problems with (missing functionality) lmk what you think |
|
Can one of the admins verify this patch? |
|
Hey @julek-wolfssl before I fix anything here since I wasn't 100% sure this is correct implementation I'd like if you can verify on your end if it's fine or not before I continue editing this |
dgarske
left a comment
dgarske
left a comment
There was a problem hiding this comment.
@Roy-Carter looks like issue with --enable-asynccrypt "FAIL: scripts/unit.test"
julek-wolfssl
left a comment
julek-wolfssl
left a comment
There was a problem hiding this comment.
I would like a test to check that the connection still completes after a call to SSL_clear_chain_certs.
Roy-Carter
force-pushed
the
feature/missing_openssl_functionality
branch
from
3e105d7 to
5f597d8
Compare
Regarding this without the PR fix of #10517 there's a segfault in clear , so just for the sake of the test + fix i've added the two bugs I encountered as part of the test because wolfSSL_add0_chain_cert never increments certChainCnt (CTX version does), and clear_chain_certs never resets it thus TLS 1.3 send walks a freed/NULL |
There was a problem hiding this comment.
Pull request overview
Adds three OpenSSL compatibility layer functions needed for application migration: SSL_CIPHER_find, sk_SSL_CIPHER_delete, and SSL_clear_chain_certs, with unit tests and a corresponding fix that increments ssl->buffers.certChainCnt when wolfSSL_add0_chain_cert succeeds.
Changes:
- New
wolfSSL_SSL_CIPHER_findwalks the SSL's cipher list to match by a 2-byte wire-format suite id. - New
wolfSSL_sk_SSL_CIPHER_deleteremoves a cipher from a stack at a given index, returning a heap copy. - New
wolfSSL_clear_chain_certsfrees the SSL's added chain certs and resets ownership;add0_chain_certnow bumpscertChainCnt.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| wolfssl/ssl.h | Public prototypes for the three new APIs |
| wolfssl/openssl/ssl.h | OpenSSL-style macro aliases for the three new APIs |
| src/ssl.c | Implementation of wolfSSL_SSL_CIPHER_find |
| src/ssl_sk.c | Implementation of wolfSSL_sk_SSL_CIPHER_delete using wolfSSL_sk_pop_node |
| src/ssl_load.c | Implementation of wolfSSL_clear_chain_certs and certChainCnt++ fix in add0_chain_cert |
| tests/api.c | New unit tests for the three APIs and a handshake-after-clear test |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
julek-wolfssl
left a comment
julek-wolfssl
left a comment
There was a problem hiding this comment.
Please address the copilot comment and fix the CI. Thanks.
wolfSSL_SSL_CIPHER_find - find cipher by 2 bytes in wired like openssl wolfSSL_sk_SSL_CIPHER_delete - remove cipher at given index SSL_clear_chain_certs
…o seen under this test
Roy-Carter
force-pushed
the
feature/missing_openssl_functionality
branch
from
1218b1c to
efbe5f7
Compare
|
@julek-wolfssl can we re run workflow see if its working now ? |
|
Description
Implement needed as part of migrating from OpenSSL -> WolfSSL
wolfSSL_SSL_CIPHER_find - find cipher by 2 bytes in wired like openssl
wolfSSL_sk_SSL_CIPHER_delete - remove cipher at given index
SSL_clear_chain_certs
Testing
created unitests .
Checklist